Skip to content

[ClangImporter] Import underlying Clang locations#39701

Closed
bnbarham wants to merge 1 commit intoswiftlang:mainfrom
bnbarham:import-clang-locations
Closed

[ClangImporter] Import underlying Clang locations#39701
bnbarham wants to merge 1 commit intoswiftlang:mainfrom
bnbarham:import-clang-locations

Conversation

@bnbarham
Copy link
Copy Markdown
Contributor

Rather than producing diagnostics with unknown locations and generated
Swift code, import the underlying header locations from Clang instead.

Resolves rdar://84127604

Rather than producing diagnostics with unknown locations and generated
Swift code, import the underlying header locations from Clang instead.

Resolves rdar://84127604
Comment on lines +93 to +94
// TODO: The LineTable is obstensibly part of the "internals", should we be
// relying on it here?
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entries() is a function I've added on the LLVM side so I'd need to merge that before this one, but I'd like some opinions on this before doing that.

I'd like to avoid creating a virtual file for every line within a line directive. The line table should be complete in all cases other than diagnostics, so this seemed like the easiest option - but if anyone has a better idea I'd be happy to implement that instead :)

func foo(_: SwiftConstrGenericNameAlias<String>) {
// expected-error@-1 {{'SwiftConstrGenericNameAlias' requires that 'String' inherit from 'NSNumber'}}
// expected-note@-2 {{requirement specified as 'T' : 'NSNumber' [with T = String]}}
// FIXME-note@-2 {{requirement specified as 'T' : 'NSNumber' [with T = String]}}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a few tests like this - should I just switch over to using FileCheck? May be worth adding some more tests to explicitly test this case as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant